perf(auth): cache account pool store scans#163
Merged
amondnet merged 6 commits intoJul 15, 2026
Conversation
Avoid repeated credential-file reads while the account store directory remains unchanged, while preserving mtime-based no-restart discovery. Refs #151
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces an in-memory caching mechanism for account directory scans in src/auth/shared.rs using the directory's modification time (mtime) to optimize steady-state request performance by avoiding redundant file I/O. It includes the implementation of scan_cached, a process-wide cache, and corresponding unit tests. The review feedback suggests refactoring scan_cached to be asynchronous and wrapping the synchronous directory scanning in tokio::task::spawn_blocking to prevent blocking the async runtime worker threads.
Contributor
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Review of the mtime-keyed account-pool scan cache surfaced a latent cross-provider hazard and several inaccurate comments: - Key the shared cache by (provider, dir), not dir alone, so two stores pointed at the same directory (SHUNT_CLAUDE_ACCOUNTS_DIR == SHUNT_CODEX_ACCOUNTS_DIR) can never serve one provider's scan (e.g. Codex's UUID-less entries) to the other. - Scope the zero-read claim to account-list discovery (a full request still reads the selected account file), reword the stale per-request scan line, and frame directory mtime as a best-effort invalidation signal (coarse-resolution filesystems can defer a re-scan one tick). - Soften the concurrent-miss comment: an entry is only served while its stored mtime matches, so a stale write is re-scanned away next request. - Strengthen the invalidation test to prove the refreshed account set replaces the cached one (not just that it re-scans) and to assert a second provider keeps its own entry at the same path. Refs #151
…tion test Second review pass on the cache comments: - Note that provider_label now also partitions the scan cache, not just the error text. - Correct the coarse-mtime caveat: a change sharing the cached scan's timestamp goes unnoticed until a later change advances the mtime (not merely "within one clock tick"). - State the concurrent-miss guarantee precisely: snapshots may differ and the last write wins, but an entry is served only while its stored mtime equals the mtime the request sampled, so a stale write is re-scanned away once a request observes a different mtime. - Strengthen the test with a second same-provider call proving the entry is retained (cache hit), not just isolated from the other provider. Refs #151
amondnet
marked this pull request as ready for review
July 15, 2026 11:16
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Greptile SummaryThis PR adds a process-wide mtime-keyed cache for account-pool store scans in
Confidence Score: 5/5Safe to merge; the change is a pure performance optimization with no observable behavior change. The caching logic is correct: mtime is sampled before the scan, the lock is dropped during I/O to avoid blocking concurrent hits, and the no-mtime bypass ensures correctness on every platform. Concurrency trade-offs are well-documented in code comments. No files require special attention. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant R as Pooled Request
participant B as spawn_blocking
participant FS as Filesystem
participant C as scan_cache (Mutex)
R->>B: resolve_pool_accounts(provider, dir, scan)
B->>FS: fs::metadata(dir)
alt dir absent (NotFound)
FS-->>B: Err(NotFound)
B-->>R: Ok([])
else stat succeeds
FS-->>B: Ok(meta) → mtime
B->>C: lock → get((provider, dir))
alt "cache hit (stored mtime == current mtime)"
C-->>B: accounts.clone()
B-->>R: Ok(cached accounts)
else cache miss
C-->>B: None or stale mtime
Note over B: drop lock before I/O
B->>FS: scan() → read_dir + per-file reads
FS-->>B: accounts
B->>C: "lock → insert((provider, dir), {mtime, accounts})"
B-->>R: Ok(fresh accounts)
end
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant R as Pooled Request
participant B as spawn_blocking
participant FS as Filesystem
participant C as scan_cache (Mutex)
R->>B: resolve_pool_accounts(provider, dir, scan)
B->>FS: fs::metadata(dir)
alt dir absent (NotFound)
FS-->>B: Err(NotFound)
B-->>R: Ok([])
else stat succeeds
FS-->>B: Ok(meta) → mtime
B->>C: lock → get((provider, dir))
alt "cache hit (stored mtime == current mtime)"
C-->>B: accounts.clone()
B-->>R: Ok(cached accounts)
else cache miss
C-->>B: None or stale mtime
Note over B: drop lock before I/O
B->>FS: scan() → read_dir + per-file reads
FS-->>B: accounts
B->>C: "lock → insert((provider, dir), {mtime, accounts})"
B-->>R: Ok(fresh accounts)
end
end
Reviews (2): Last reviewed commit: "docs(auth): reflow account scan cache co..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
amondnet
deleted the
amondnet/perf-cache-claude-account-pool-store-scan-avoid
branch
July 15, 2026 11:42
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Cache account-pool store scans by the store directory's modification time. Previously, every pooled request traversed
resolve_pool_accounts→scan_accounts→read_account_uuid, performing one directory read plus one credential-file read and JSON parse per account. The shared process-wideauth::shared::scan_cachedhelper now serves the previous scan when the directory mtime is unchanged, reducing steady-state discovery to one metadata stat and zero credential-file reads.The existing metadata lookup from #118 supplies the cache signal, so this adds no extra stat. Directory changes still invalidate the cache for no-restart account discovery, scans remain on
spawn_blocking, absent stores retain the cheapNotFoundshort-circuit, and platforms without an mtime bypass the cache. Claude and Codex use separate store paths and can both benefit, while the primary improvement is avoiding Claude UUID reads.Closes #151
Part of #149 (Tier 1).
Milestone / spec
Performance follow-up tracked by #149 (Tier 1) and #151. This internal refactor does not change observable behavior, configuration, endpoints, CLI behavior, or provider/model semantics, so no
README.md,docs/, orsite/update is needed.Checklist
cargo buildpassescargo testpasses (new behavior is covered; tests run without network/loopback where possible)cargo clippy --all-targets -- -D warningscleancargo fmt --all --checkcleandocs/updated if this change deviates from itREADME.md/site/as applicable (wiki/is generated; don't hand-edit)Notes for reviewers
scan_cached_serves_cache_until_mtime_changes_and_bypasses_without_mtimeinjects mtimes directly to verify cache hits, invalidation, and no-mtime bypass behavior.resolve_pool_accounts_caches_scan_across_unchanged_requestsverifies against the real filesystem that an unchanged second request does not re-scan.cargo fmt --all --check,cargo clippy --all-targets --all-features -- -D warnings, andcargo test --all-features --workspace(529 library tests plus all integration tests, includingmulti_account).auth::sharedclosely.Summary by cubic
Caches pooled-account store scans by store-directory mtime to skip repeat credential-file reads during discovery; biggest win is avoiding Claude UUID reads. The cache is keyed by provider and directory; unchanged stores now cost one stat and zero credential-file reads, with behavior unchanged.
(provider, dir)and invalidated by the store’s mtime.spawn_blocking.Written for commit 299f489. Summary will update on new commits.